feat: add extra sstore benchmark cases #1774
Conversation
LouisTsai-Csie
added
A-test-benchmark
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1774 +/- ##
================================================
+ Coverage 53.45% 87.06% +33.61%
================================================
Files 743 541 -202
Lines 44076 32832 -11244
Branches 3891 3015 -876
================================================
+ Hits 23559 28586 +5027
+ Misses 20306 3598 -16708
- Partials 211 648 +437
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
spencer-tb
left a comment
spencer-tb
left a comment
There was a problem hiding this comment.
Thanks some comments! :)
| @pytest.mark.parametrize("slot_count", [50, 100]) | ||
| @pytest.mark.parametrize("use_access_list", [True, False]) | ||
| @pytest.mark.parametrize( | ||
| "contract_size", | ||
| [ | ||
| pytest.param(0, id="just_created"), | ||
| pytest.param(1024, id="small"), | ||
| pytest.param(12 * 1024, id="medium"), | ||
| pytest.param(24 * 1024, id="xen"), | ||
| ], | ||
| ) | ||
| @pytest.mark.parametrize("sloads_before_sstore", [True, False]) | ||
| @pytest.mark.parametrize("num_contracts", [1, 5, 10]) | ||
| @pytest.mark.parametrize( | ||
| "initial_value,write_value", | ||
| [ | ||
| pytest.param(0, 0, id="zero_to_zero"), | ||
| pytest.param(0, 0xDEADBEEF, id="zero_to_nonzero"), | ||
| pytest.param(0xDEADBEEF, 0, id="nonzero_to_zero"), | ||
| pytest.param(0xDEADBEEF, 0xBEEFBEEF, id="nonzero_to_nonzero"), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
This will create 384 seperate tests. Do we need this many here? Maybe we should remove some parameterization. I say this with the benchmark release process taking a long time in mind.
Maybe contract size and num contracts can have one element removed from each.
If these are required in your opinion please keep them. Just a thought.
LouisTsai-Csie
force-pushed
the
feat/extra-sstore-benchmark
branch
from
3c7d208 to
1072748
Compare
LouisTsai-Csie
force-pushed
the
feat/extra-sstore-benchmark
branch
from
1072748 to
0f4e7eb
Compare
| pytest.param(0, id="just_created"), | ||
| pytest.param(1024, id="small"), | ||
| pytest.param(12 * 1024, id="medium"), | ||
| pytest.param(24 * 1024, id="xen"), |
There was a problem hiding this comment.
To make this compatible with Kamil's tool you should set IDs that have the name of the test _ contract name.
See my file as an example to understand the naming convention: https://gist.github.com/CPerezz/51e292676f2420cfb601d251a7ad72e7
| benchmark: | ||
| evm-type: benchmark | ||
| fill-params: --fork=Prague --gas-benchmark-values 1,10,30,45,60,100,150 -m "benchmark and not state_test" ./tests/benchmark | ||
| fill-params: --fork=Prague --gas-benchmark-values 5,10,30,45,60,100,150 -m "benchmark and not state_test" ./tests/benchmark |
There was a problem hiding this comment.
I think 1 is needed. But can be maybe done on the side?
I recall Maria wanted it
4 participants
🗒️ Description
Add extra SSTORE benchmark cases, more details linked in the issue #1755 .
🔗 Related Issues or PRs
Issue #1755
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_frommarker.Cute Animal Picture